Skip to content
This repository has been archived by the owner on May 19, 2020. It is now read-only.

chore(*): comment out underline-related code - I97 #99

Merged
merged 2 commits into from
Oct 5, 2019
Merged

chore(*): comment out underline-related code - I97 #99

merged 2 commits into from
Oct 5, 2019

Conversation

cmccandless
Copy link
Contributor

Issue #97

Remove underline button from toolbar

Changes

  • Comment out references to underline (left as placeholders for whenever #98 is resolved)

Related Issues

  • Issue #98

Resolves #97

@cmccandless cmccandless force-pushed the remove-underline-from-toolbar branch 2 times, most recently from 99f1a81 to 0fe42db Compare October 3, 2019 19:24
dselman
dselman approved these changes Oct 3, 2019
View reviewed changes
Copy link
Contributor

@dselman dselman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

jolanglinais
jolanglinais suggested changes Oct 3, 2019
View reviewed changes
Copy link
Member

@jolanglinais jolanglinais left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were you able to test these, in terms of seeing if the CSS Grid layout still works? It was specifically designed to have the underline button there.

@cmccandless
Copy link
Contributor Author

Works just fine:
image
Bold, Italic, etc buttons still work as well.

@cmccandless
Copy link
Contributor Author

If anything, there is a slightly noticeable extra spacing created by the grid.

@cmccandless
Copy link
Contributor Author

Latest commit fixes the extra spacing:
image

@jolanglinais
Copy link
Member

jolanglinais commented Oct 3, 2019 •
edited
Loading

Right, I think we also need to be editing the CSS to accommodate for this. I hope we get a solution in for handling underline soon, but in case we don't, we want the toolbar to not look like something is missing. Ah, didn't see your newest commit.

@cmccandless
Copy link
Contributor Author

@irmerk If there are no other changes to request, can you please mark approval?

@jolanglinais
Copy link
Member

You'll need to correct the DCO on your last commit.

@cmccandless cmccandless force-pushed the remove-underline-from-toolbar branch from a9ef679 to 0a373e9 Compare October 4, 2019 13:23
@cmccandless cmccandless force-pushed the remove-underline-from-toolbar branch from 0a373e9 to 19539ef Compare October 4, 2019 16:01
@cmccandless
Copy link
Contributor Author

@irmerk done

@jolanglinais jolanglinais changed the title Comment out underline-related code chore(*): comment out underline-related code - I97 Oct 5, 2019
jolanglinais
jolanglinais approved these changes Oct 5, 2019
View reviewed changes
Copy link
Member

@jolanglinais jolanglinais left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, this is a help! Now just to get HTML support for underline.

@jolanglinais jolanglinais merged commit 251bd5d into accordproject:master Oct 5, 2019
@cmccandless cmccandless deleted the remove-underline-from-toolbar branch October 6, 2019 02:03
@jolanglinais jolanglinais mentioned this pull request May 13, 2020
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jolanglinais jolanglinais jolanglinais approved these changes

@dselman dselman dselman approved these changes

Assignees

@cmccandless cmccandless

Labels
Difficulty: Medium Hacktoberfest by DigitalOcean and DEV Type: Regression 🖐
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove Underline Button from Toolbar
3 participants
@cmccandless @jolanglinais @dselman